Repository navigation
fix(shader): record BDA faults in hull programs instead of refusing them - #4222
oneandonlydean merged 3 commits into
Conversation
47446b9 to
22be94e
Compare
22be94e to
d537af1
Compare
d537af1 to
699733d
Compare
|
Rebase handoff — this PR is the 5th link of the hull/tessellation chain (it contains #4172 and #4201 underneath its own commit), so I stacked the rebase on the already-rebased chain: Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up (whole chain): Head is |
699733d to
3f165b6
Compare
3f165b6 to
c4b02f0
Compare
c4b02f0 to
3a72fa3
Compare
3a72fa3 to
f33872b
Compare
|
Quick note: I restacked this on the new #4172 and #4201 heads. The only conflict was the order of the test calls in |
A wave64 hull program narrows EXEC with s_and_saveexec_b64 and restores it later, so its exec words reach the lane-bit test as ANDs with the host ballot, as loop or join phis, and through separate ballots for the two halves. Tessellation lowering refused them as "uses a lane mask other than its own lane's bit". The own-lane bit of a mask pair is now rebuilt from the words' operations (ballots, bitwise ops, selects, phis, constants); both halves must have the same shape and ballot the same predicate. Dead hull cycles such as a saved-exec loop are removed before the remaining-ballot check, so a mask whose other lanes are read (s_bcnt1_i32_b64 exec) is still refused. The words are replaced through identities and block conditions are resolved, so a branch on a compare copied to VCC keeps its condition. Local and domain programs keep their exec words for the half-wave matcher. Block conditions fix and HullCompareBranch test from EagleBearerYS.
… data Hull programs whose relative patch index (v1 bits 7:0) reached anything but a tessellation ring or LDS address threw, because the lowering runs every Vulkan patch as relative patch 0. The quad terrain hull 0x80002fc700 in Ghost of Yotei is one, and so is the domain 0x8068fbd900 (v7). Such hulls and domains now take (PrimitiveID mod GE_CNTL.PRIM_GRP_SIZE) mod VGT_LS_HS_CONFIG.NUM_PATCHES, as if hull threadgroups took NUM_PATCHES patches at a time out of each primitive group; both stages see the same PrimitiveID for a patch. This holds only when every instance starts a new threadgroup, so it needs GE_CNTL.BREAK_WAVE_AT_EOI (bit 18); with the bit clear the instances share threadgroups and such programs still throw. PRIM_GRP_SIZE and BREAK_WAVE_AT_EOI join the tessellation configuration (draw keys, hull and domain cache keys, request version 24). A missing GE_CNTL throws, and so do local programs, whose relative vertex index depends on vertex reuse that Vulkan does not expose.
Every hull program with a BDA access was refused with "BDA fault termination requires a barrier-safe tessellation-control execution protocol". The refusal predates BdaInvocationsMayStop: a program with s_barrier no longer stops a faulting invocation; the fault is recorded, the read returns zero and every invocation reaches the barrier. In a hull, s_barrier is the only control barrier the backend emits after the guest code starts, so that rule already covers it, and a hull without one can stop an invocation without stranding a barrier. The tessellation test draws hulls that scale their levels by a BDA read, with and without s_barrier, and requires an unmapped read to report a tessellation-control fault (not on llvmpipe, which runs hull loads that the unmapped-address check skips). The contract test now accepts a hull with s_barrier.
f33872b to
c1e4a0b
Compare
What
Stacked on #4201; please review the last commit only.
A hull (tessellation-control) program with a BDA read fails to compile with "BDA fault termination requires a barrier-safe tessellation-control execution protocol". The refusal dates from the first BDA commit and is now redundant:
BdaInvocationsMayStopalready switches any program that containss_barrierto the non-stopping fault protocol, and a hull's only barriers come from guests_barrier(EmitBarrier); the LDS-lock barrier runs before any guest code and wave-LDS barriers are compute-only.s_barrierkeeps every invocation to its barriers; a hull without one may stop a faulting invocation, since no barrier can be stranded.BdaContractsasserted the refusal and now accepts a hull withs_barrier.PPSA26344's terrain hull 0x80002fc700 has no
s_barrierand reads guest memory through BDA. With this change, its replayed request recompiles completely (388,903 SPIR-V words) onceGE_CNTL.BREAK_WAVE_AT_EOIis set in the request; the title clears that bit, which #4201 refuses and #4450 handles.Tested
Based on
mainf265cd5 plus #4172 and #4201. Windows 11 (MinGW-w64 GCC 15.2.0) and Linux: configure -DBUILD_TESTING=ON, targeted test builds, ctest.agc_driver_tessellation,agc_shader_memory,agc_driver_graphics,agc_tessellation_factor_ringandagc_driver_bda_devicepass (5/5)main: the same 5 tests pass (5/5)log_macros, which fails onmaintoo; Linux (upstream's job in WSL) merged withmain5a81f60: 1089/1091,agc_driver_bc_block_copyandagc_driver_depth_compare_copyfail on thatmainas wellagc_driver_tessellationgains two hulls that scale their output by a float read through BDA, with and withouts_barrier: both render correctly, and an unmapped read reportsBDA access failed ... stage=7. Both this andagc_driver_bda_devicefail on #4201's head with the refusal. The unmapped-read draws are skipped on llvmpipe, which segfaults on them although the loads are guarded byif (physical != 0)and removing just those guarded loads stops the crash; the mapped-read draws run there.Not tested: console behaviour.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)